Skip to content

TerminalApp: foreground target window in FocusProtocolPane#353

Merged
DDKinger merged 1 commit into
mainfrom
dev/yuazha/session-mgmt-crosswindow-focus
Jun 24, 2026
Merged

TerminalApp: foreground target window in FocusProtocolPane#353
DDKinger merged 1 commit into
mainfrom
dev/yuazha/session-mgmt-crosswindow-focus

Conversation

@DDKinger

@DDKinger DDKinger commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Cross-window focus_pane (Enter on a Live session in the session-management view) could fail to bring the target window to the foreground when that window already had the target pane focused.

Repro

  1. Two windows: window1 and window2.
  2. window1 hosts session1 (in one of its panes); window2 hosts session2.
  3. From window2's session-management view, press Enter on session1.
  • ❌ When window1 was already focused on session1's own pane (e.g. the top shell pane), window1 did not come forward.
  • ✅ When window1 was focused on a different pane (e.g. the agent pane), it worked.

Root cause

TerminalPage::FocusProtocolPane only moved XAML focus within the target window via _SetFocusedTab / FocusPane. Those no-op when the pane is already focused, and the OS window previously only came forward as an accidental side effect of the focus actually transitioning. When the target pane was already focused, no transition happened → no activation → the window stayed in the background.

Fix

Raise SummonWindowRequested once the target pane is found, mirroring the desktop-notification activation path in TabManagement.cpp. The window is then reliably brought to the foreground regardless of its prior focus state. SummonWindow(toggleVisibility=false)_globalActivateWindow performs the activation; it is intra-process (the COM server runs inside WindowsTerminal.exe), so no AllowSetForegroundWindow is required.

Test plan

  • Full solution build (Debug/x64): 0 Error(s); WindowsTerminal.exe + CascadiaPackage link and pack cleanly.
  • Runtime: two windows; Enter on a session whose pane is already focused in its window → window surfaces and the pane is focused.

focus_pane (Enter on a Live session in the session management view) could fail to surface the target window when that window already had the target pane focused. FocusProtocolPane only moved XAML focus *within* the window via _SetFocusedTab/FocusPane — which no-op when the pane is already focused — and relied on the focus transition as an accidental side effect to bring the OS window forward.

Symptom: focusing session1 (in window1) from window2 worked only when window1's focus was on a different pane (e.g. the agent pane), but not when window1 was already focused on session1's own pane.

Fix: raise SummonWindowRequested once the target pane is found, mirroring the desktop-notification activation path in TabManagement.cpp, so the window is reliably brought to the foreground regardless of its prior focus state.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 03:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes cross-window focus_pane activation so that focusing a protocol pane in another window reliably brings the target OS window to the foreground even when that window already had the target pane focused (i.e., when intra-window XAML focus operations would otherwise no-op).

Changes:

  • Raise SummonWindowRequested inside TerminalPage::FocusProtocolPane once the target pane is located, ensuring OS-level window activation.
  • Preserve the existing tab/pane focusing logic (including the stashed agent-pane restore path) after summoning.

@DDKinger DDKinger merged commit 1f205dd into main Jun 24, 2026
8 of 10 checks passed
vanzue added a commit that referenced this pull request Jul 3, 2026
Second review pass over the release checklist against the v0.1.2 release notes:

- Added a [new] coverage marker to the legend and tagged all 12 genuinely-new v0.1.2 test cases with it (the two reworded existing cases — F5 refresh, ACP-sourced historical — are corrections, not new cases, so they stay untagged).
- Added 5 more previously-missing cases surfaced by a closer read:
  - FRE execution-policy detection is correct / no false-block on probe timeout (#336/#338/#309)
  - wta-master death is a consistent degraded state requiring /restart, no split-brain (#329)
  - Session titles are clean (no bare '# AGENTS.md instructions' heading) (#355)
  - Focus brings the target window to the foreground (#353)
  - Agent-created terminals inherit the active pane profile (#366, closes #351)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vanzue added a commit that referenced this pull request Jul 3, 2026
* docs(release-check-list): sync with v0.1.2 features

Reviewed the release checklist against the v0.1.2 release notes and fixed two outdated statements plus added seven missing test cases for new features.

Outdated statements corrected:
- Session view refresh: now F5 on-demand re-scan (incl. WSL distros started after launch), not gated on hooks (#344).
- Historical state: history is now sourced from ACP session/list, not on-disk file parsing (#365).

Missing test cases added:
- Alt+V clipboard image paste into agent chat (#354)
- GitHub Enterprise Copilot sign-in (#362)
- Bash/WSL shell integration incl. set -u safety (#340)
- Shells self-report identity via OSC 9001;ShellType + nested-shell autofix targeting (#345)
- Environment-aware answers/fixes: investigate PATH before answering/autofixing (#306)
- WSL distro sessions visible and resumable in the correct distro (#323)
- Agent panes not persisted into saved window layout (#360/#275)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(release-check-list): tag new v0.1.2 cases with [new] + add 5 more

Second review pass over the release checklist against the v0.1.2 release notes:

- Added a [new] coverage marker to the legend and tagged all 12 genuinely-new v0.1.2 test cases with it (the two reworded existing cases — F5 refresh, ACP-sourced historical — are corrections, not new cases, so they stay untagged).
- Added 5 more previously-missing cases surfaced by a closer read:
  - FRE execution-policy detection is correct / no false-block on probe timeout (#336/#338/#309)
  - wta-master death is a consistent degraded state requiring /restart, no split-brain (#329)
  - Session titles are clean (no bare '# AGENTS.md instructions' heading) (#355)
  - Focus brings the target window to the foreground (#353)
  - Agent-created terminals inherit the active pane profile (#366, closes #351)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(e2e): add master-death degraded-state integration test (#329); drop rolled-back WSL session checklist item

Adds Feature.AgentMasterDeath.Tests.ps1: kills this app's wta-master out from under a live helper and asserts the full #329 contract end-to-end — the pane leaves Connected, the / popup is filtered to only /restart, NO master is silently respawned while degraded (anti-split-brain), and /restart brings up exactly one fresh master and reconnects. Verified passing live against the deployed dev package.

Also removes the '[new] WSL distro sessions are visible and resumable' checklist item (#323) since WSL in-distro session support is being rolled back for v0.1.2, and drops the WSL example from the F5 refresh item to match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address Copilot review: tighten master-count assertion and clarify Alt+V behavior

- Feature.AgentMasterDeath.Tests.ps1: assert exactly one wta-master while connected (-Be 1, was -BeGreaterThan 0) so the connected-state gate itself catches a split-brain regression. Re-verified passing live.

- release-check-list.md: correct the Alt+V item — Alt+V queues the image until the next prompt, and an unsupported agent / empty clipboard surfaces a clear system message rather than being a silent no-op.

The [WSL-<distro>] raw-angle-bracket comment is already resolved: that checklist item was removed with the WSL session rollback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address Copilot review round 2: block /stop in degraded popup test, release-agnostic [new] marker, spelling fix

- Feature.AgentMasterDeath.Tests.ps1: add /stop to the blocked-slash-command list so a regression that offers /stop while transport-lost is caught (it's a real CommandKind::Stop). Re-verified passing live.

- release-check-list.md: reword the [new] marker in release-agnostic terms and say when to clear it (was hard-coded to v0.1.2); fix forbidden spelling non-existent -> nonexistent (check-spelling CI).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants